Skip to content

Compiled ToStrings under -reflectionfree for DUs and Records#19976

Open
charlesroddie wants to merge 17 commits into
dotnet:mainfrom
charlesroddie:DUsRecordsCompiledToStrings
Open

Compiled ToStrings under -reflectionfree for DUs and Records#19976
charlesroddie wants to merge 17 commits into
dotnet:mainfrom
charlesroddie:DUsRecordsCompiledToStrings

Conversation

@charlesroddie

@charlesroddie charlesroddie commented Jun 21, 2026

Copy link
Copy Markdown

This PR implements ToString on user-defined DUs and records, under the --reflectionfree switch, by delegating into the equivalent of LanguagePrimitives.anyToStringShowingNull, as is currently used by Option. Previously only the type name was shown. The new ToString is AOT/trim friendly, provided that the inner types have AOT/trim-friendly ToString methods.

It also implements ToString on Result and Choice types, and consolidates behaviour to match Option.

Fixes fsharp/fslang-suggestions#919

Behavioural changes (ungated)

  • Ok(0) now renders as "Ok(0)" (previously "Ok 0"), matching Some(0) which renders as "Some(0)".

Behavioural changes (subject to --reflectionfree)

  • Records and DUs now generate real ToStrings instead of just the type name. The behaviour matches Option for the internal rendering, and this differs from non-reflection-free mode (sprintf "%+A") in the following ways:
    • The floating point number 5. renders as 5.
    • Records render on a single line, avoiding the complex indentation logic in sprintf "%A".
    • Anonymous records render with {| ... |} syntax instead of { ... }

While the exact code of Option couldn't be used directly (since anyToStringShowingNull is internal to FSharp.Core), it was easy to match it, and there are some tests that the behaviour is the same, so that if there are any changes to the above in Option (which there is some ongoing discussion about, e.g. over null rendering), they are synced up with other DU non-reflection-based rendering.

It is obviously easy to make the changes to --reflectionfree mode a decisive improvement, since we are starting with nothing. However the intention would be that the behaviour is good enough to switch as a default in a future version of F#.

Note: this PR and #19971 add an identical v_string_operator_info and mkCallStringOperator.

charlesroddie and others added 9 commits June 21, 2026 20:06
Adds string_operator_info / mkCallStringOperator so generated code can call
Operators.string. These lines are duplicated by the interpolated-string PR
(dotnet#19971); kept identical there so a future merge resolves cleanly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Under --reflectionfree the union ToString previously emitted nothing, so
DUs fell back to Object.ToString() (the namespace-qualified type name).
Instead generate a match over the cases that builds "CaseName(f0, f1, ...)"
using the 'string' operator on each field, via a TypedTree expression fed
to CodeGenMethodForExpr. This recurses naturally into nested unions and is
reflection-free. The default (sprintf "%+A") path is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "concatenate a list of string exprs, picking the cheapest String.Concat
overload by arity" pattern was duplicated in CheckExpressions (interpolation
lowering) and the optimizer, and our new union ToString used the array
overload unconditionally. Extract mkStringConcat into TypedTreeOps.ExprOps
and route all three through it. This also lets single-field union cases emit
Concat3 instead of allocating a string[] (IlxGen runs after the optimizer, so
nothing else would collapse that array form).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The match-based ToString body is a TypedTree expression codegen'd via
CodeGenMethodForExpr, but it was built with `eenv`, which lacks the tycon's
type parameters. For generic unions this produced wrong IL: the wrong case
branch (always the null-as-true-value case) or a NullReferenceException for
single-case unions. Use `eenvinner` (the per-tycon environment) so the
generic method body resolves its type parameters. The old sprintf path was
unaffected because it emits raw IL off the pre-built ilThisTy.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
To make a generated union ToString consistent with how option/list format
their contents (LanguagePrimitives.anyToStringShowingNull), format each field
as: if (box field) is non-null then 'string field' else "null". Previously a
null field rendered as "" (the 'string' operator's null behaviour). Generated
inline rather than calling anyToStringShowingNull, which is internal to
FSharp.Core and so not callable from user-compiled code.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Normalize union declarations to a leading '|', use System.Console.WriteLine
instead of printfn (the printf machinery is what these changes move away
from), and make the null-field test compare the union's rendering directly
against option's rather than asserting a fixed string.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Result and Choice had no ToString override, so they fell back to the
compiler-generated sprintf "%+A" one, which uses reflection. Give them
hand-written overrides mirroring option/list (String.Concat +
anyToStringShowingNull), e.g. Ok 5 -> "Ok(5)", Choice1Of2 7 -> "Choice1Of2(7)".
This is reflection-free / AOT-friendly and consistent with option's "Some(x)"
rendering. Note: this changes the observable ToString of Result/Choice from
the "%A"-style "Ok 5" to "Ok(5)".

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Records previously fell back to Object.ToString() (the namespace-qualified
type name) under --reflectionfree. Generate "{ F1 = v1; F2 = v2 }" on a single
line (no line breaks, unlike sprintf "%+A"), with fields formatted like union
fields (null -> "null", otherwise via 'string'). Factor the shared field
formatter and ToString-method emission out of the union path. The default
(sprintf "%+A") path is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Result and Choice`2..7 now declare an explicit ToString() override, so they
appear in the public surface area.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev

@charlesroddie,

Caution

No release notes found for the changed paths (see table below).

Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format.

The following format is recommended for this repository:

* <Informative description>. ([PR #XXXXX](https://github.com/dotnet/fsharp/pull/XXXXX))

See examples in the files, listed in the table below or in th full documentation at https://fsharp.github.io/fsharp-compiler-docs/release-notes/About.html.

If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request.

Change path Release notes path Description
src/FSharp.Core docs/release-notes/.FSharp.Core/11.0.100.md No release notes found or release notes format is not correct

✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@charlesroddie charlesroddie marked this pull request as ready for review June 21, 2026 19:26
@charlesroddie charlesroddie requested a review from a team as a code owner June 21, 2026 19:26
@kerams

kerams commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Kickass

@github-actions github-actions Bot added the ⚠️ Affects-Compiler-Output Tooling check: PR touches IL emission or codegen label Jun 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Tooling Safety Check — Affects-Compiler-Output
Affects-Compiler-Output: IlxGen.fs, prim-types.fs, Optimizer.fs modify IL emission and FSharp.Core runtime

Generated by PR Tooling Safety Check · opus46 4.2M ·

…ionfree

Drive anonymous-record ToString through the synthetic record tycon (already
built for equality/comparison) rather than sprintf "%A", so under
--reflectionfree it renders "{| Name = value; ... |}" on a single line.
GenRecordToStringMethod now takes open/close brace strings ("{ "/" }" for
records, "{| "/" |}" for anonymous records). The default (non-reflection-free)
codegen path is unchanged and still falls back to sprintf "%+A".

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

let mdef =
mkILNonGenericVirtualInstanceMethod (
"ToString",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if there was a string override present, written by hand?

[<NoComparison;NoEquality>]
type MyDU = A of int
    with override x.ToString() = "A"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, and a test is added in a5f33ca

Comment thread src/Compiler/CodeGen/IlxGen.fs Outdated
Comment on lines +11321 to +11324
if not g.useReflectionFreeCodeGen then
GenPrintingMethod cenv eenv "ToString" ilThisTy m
else
let tinst, thisv, thise = GenToStringThis (cenv, tcref, m)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that IlxGen has two different generators for ToString, I would appreciate a more distinct naming.
Fine if it becomes longer, better to avoid confusion.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed in 7ed0d8d :

  • GenPrintingMethod → GenSprintfPrintingMethod (the reflective sprintf "%+A" path, used for both ToString and exception get_Message)
  • GenToStringMethodFromExpr → EmitToStringMethodDef.

Comment thread src/Compiler/TypedTree/TcGlobals.fs Outdated

let v_byte_operator_info = makeIntrinsicValRef(fslib_MFOperators_nleref, "byte" , None , Some "ToByte", [vara], ([[varaTy]], v_byte_ty))
let v_sbyte_operator_info = makeIntrinsicValRef(fslib_MFOperators_nleref, "sbyte" , None , Some "ToSByte", [vara], ([[varaTy]], v_sbyte_ty))
let v_string_operator_info = makeIntrinsicValRef(fslib_MFOperators_nleref, "string", None, Some "ToString", [vara], ([[varaTy]], v_string_ty))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls try to respect the "tabular" code layout here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in dd3d75c . But I hope a code formatter gets to work on this file eventually!


override x.ToString() =
match x with
| Choice1Of6 v -> String.Concat("Choice1Of6(", anyToStringShowingNull v, ")")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't' the newly added IlxGen feature codegen the same code that was now added by hand?

Thinking out loud:
Maybe this could be a separate feature (which would gent turned on automatically if you had --reflectionfree), but we could apply it even without it - either by project or by type.. ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it would, if you turned on --reflectionfree, which would then break FSharp.Core since --reflectionfree bans "%A" usage. A separate feature would work for this purpose, at the cost of more plumbing.

The bigger question is when will consumers (both FSharp.Core and end users) who don't set --reflectionfree get the compiled ToStrings in the future. Turning on the flag by default would be dangerous because of the %A ban.

Perhaps the options are:

  1. Introduce a new feature now, use it in FSharp.Core straight away, and in the future it could be on by default.
  2. Stick with --reflectionfree, and in future make it on by default but remove the %A ban in it at that point (with %A possibly getting safer in future and with trim/AOT warnings available from the regular toolchain). Once this happens these explicit overrides can be removed from FSharp.Core.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this @T-Gro :

  • Make a LanguageFeature.CompiledToStrings
  • FSharp.Core opts into this and then instead of adding manual ToStrings, this PR removes them from Option and ValueOption since they are now auto-generated.
  • The implementation checks for either LanguageFeature.CompiledToStrings or --reflectionfree

@@ -35,15 +35,91 @@ let someCode =
"""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will be posting my testing remarks.

I would like to see EmittedIL tests for the core scenarios ( Record, struct record, DU, DU with usenullastruevalue,...)

@charlesroddie charlesroddie Jun 23, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 2c108ed32 but it revealed some suboptimal codegen including boxing. Investigating.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor done in 6a2199cbd

@@ -35,15 +35,91 @@ let someCode =
"""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test with recursive dependencies.
Recursively defined types, and instances nesting each other ( let's say a tree-like type, but also with parent points).

IMO C# is adding EnsureSufficientStackSize so that it avoids a stackoverlow and rather fails with a catchable exception.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finite recursive cases added in 1a5065e7e

@@ -35,15 +35,91 @@ let someCode =
"""

[<Fact>]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if Map,Set,List were user-defined and having the newly added ToString behavior?
A hypothetical custom-list is a nice test case.

Leading to a different question - this PR brings special support to some FSharp.Core types. What about collection types?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what's special about collection types.

Generating ToStrings for classes would be possible based on primary constructor inputs, but would need a separate language suggestion I think.

// Generate all the anonymous record types mentioned anywhere in this module
for anonInfo in anonRecdTypes.Values do
mgbuf.GenerateAnonType((fun ilThisTy -> GenToStringMethod cenv eenv ilThisTy m), anonInfo)
mgbuf.GenerateAnonType((fun (ilThisTy, tycon) -> GenRecordToStringMethod(cenv, mgbuf, EnvForTycon tycon eenv, ilThisTy, mkLocalTyconRef tycon, m, "{| ", " |}")), anonInfo)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure to cover anons and struct anons in testing.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Covered in 1a5065e7e


[<Fact>]
let ``Records and DUs don't have generated ToString`` () =
let ``Classes don't have a generated ToString`` () =

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two more test dimensions:

  • DUs with and without explicit field names. (considering if explicit field names should not be part of the output)
  • Using/not using tuples for DU fields:
type MyDU = A of int * int
type MyDU2 = A of (int*int)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Covered in 1a5065e7e

Explicit field names are not part of output (like current non-reflection-free ToStrings).

charlesroddie and others added 2 commits June 22, 2026 18:52
…free

Addresses review feedback: generation is gated on `not (HasMember "ToString")`,
so a user-defined ToString on a union or record wins over the generated one.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses review feedback: distinguish the reflective sprintf path from the
structural one. GenPrintingMethod -> GenSprintfPrintingMethod (the sprintf "%+A"
ToString/get_Message), GenToStringMethodFromExpr -> EmitToStringMethodDef.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
charlesroddie and others added 3 commits June 22, 2026 19:14
Addresses review feedback: keep the column-aligned layout of the surrounding
intrinsic table. Also makes these two lines byte-identical to the same intrinsic
added by dotnet#19971, so a future merge resolves cleanly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cords and recursion

Covers DU field shapes (multiple fields vs a single tuple field), explicit
vs unnamed field names rendering identically, struct unions/records,
anonymous and struct anonymous records, and finite recursive/nesting types.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Locks in the IL emitted under --reflectionfree: each field is boxed and
rendered through Operators.ToString with a null guard, and the parts are
joined with String.Concat (array form for the record, 3-arg form for the
single-field union case). Nullary union cases return the bare case name.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The structural ToString for --reflectionfree records and unions was built in
IlxGen, after the optimizer, so its per-field 'string' operator calls were
never inlined: each value-type field was boxed and rendered through the
generic Operators.ToString, behind a null guard that is dead for a value type.

Move the generation into the type-augmentation phase (alongside
Equals/GetHashCode/CompareTo) so the body flows through the optimizer. The
'string' operator is now specialised - a value-type field renders via a direct,
allocation-free invariant-culture ToString with no boxing and no null guard
(reference fields keep the guard so null still renders as "null"). The shared
body builders live in AugmentTypeDefinitions; anonymous record types are
synthesized too late for augmentation, so they keep generating in IlxGen but
reuse the same builder.

Output is unchanged; the EmittedIL baselines are updated to the leaner IL.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@charlesroddie charlesroddie force-pushed the DUsRecordsCompiledToStrings branch from 07df74b to 6a2199c Compare June 23, 2026 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚠️ Affects-Compiler-Output Tooling check: PR touches IL emission or codegen

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

Make F# string functions fast and AOT/linker friendly

3 participants